Conversation
fix: port와 db 연결 방식 변경
WalkthroughGitHub Actions의 Cloud Run 배포 플래그 통합 및 컨테이너 환경 변수에 Changes
Sequence Diagram(s)sequenceDiagram
participant User as 사용자
participant Browser as 브라우저
participant App as Spring 애플리케이션
participant Security as Spring Security
User->>Browser: `#btn-logout` 클릭
Browser->>Browser: logout() 실행\n(헤더 메타에서 _csrf 읽음)
Browser->>App: POST /logout (폼 제출, CSRF 포함)
Note over Security,App: (prod) SecurityFilterChain: HTTPS 요구 검사\n(Can't proceed if not secure)
Security->>App: CSRF 검증 및 로그아웃 처리
App->>Browser: 응답(세션 무효화 / 리다이렉트)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/main/resources/application.properties (1)
14-14: H2 콘솔 비활성화가 개발 환경에 미치는 영향을 확인하세요.H2 콘솔을 비활성화하는 것은 프로덕션 환경의 보안에는 좋지만, 로컬 개발 시 데이터베이스 디버깅에 영향을 줄 수 있습니다. 개발 환경에서는 H2 콘솔이 필요할 수 있으므로, 프로파일별 설정(예: application-dev.properties에서 enabled=true)을 고려하세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/cd.yml(1 hunks)Dockerfile(1 hunks)src/main/java/com/precourse/openMission/config/auth/SecurityConfig.java(1 hunks)src/main/java/com/precourse/openMission/web/GlobalModelAdvice.java(1 hunks)src/main/resources/application.properties(1 hunks)src/main/resources/static/js/app/index.js(3 hunks)src/main/resources/templates/layout/header.mustache(2 hunks)
🔇 Additional comments (7)
.github/workflows/cd.yml (1)
68-68: Cloud Run 최소 인스턴스 설정 검토
--min-instances=1설정으로 콜드 스타트를 방지하고 있습니다. 이는 좋은 결정이지만, 필요에 따라 트래픽 패턴과 비용을 고려하여 조정할 수 있습니다.src/main/java/com/precourse/openMission/web/GlobalModelAdvice.java (1)
13-19: CSRF 토큰 전역 노출 구현이 올바릅니다.Spring Security의 CSRF 토큰을 모든 컨트롤러의 뷰에 자동으로 노출하는 구현이 정확합니다.
@ControllerAdvice와@ModelAttribute를 활용하여 중복 코드 없이 모든 템플릿에서 CSRF 토큰에 접근할 수 있도록 했습니다.src/main/resources/templates/layout/header.mustache (3)
6-8: CSRF 메타 태그 구현이 정확합니다.GlobalModelAdvice에서 노출된 CSRF 토큰을 메타 태그로 올바르게 추가했습니다. 이를 통해 클라이언트 측 JavaScript(index.js)에서 CSRF 토큰에 접근하여 안전한 로그아웃 및 기타 POST 요청을 수행할 수 있습니다.
26-26: JavaScript 기반 로그아웃 구현이 올바릅니다.로그아웃 버튼을 앵커 링크로 변경하고 JavaScript에서 CSRF 토큰을 포함한 POST 요청을 처리하는 방식이 적절합니다. index.js의 로그아웃 핸들러와 올바르게 연동됩니다.
5-5: 리뷰 의견이 코드베이스의 실제 상태와 일치하지 않습니다.검증 결과:
- 현재 템플릿들은 Bootstrap 4.3.1과 완전히 호환되는 코드로 작성되어 있습니다
- Bootstrap 4 필수 요소들이 올바르게 구성됨: jQuery 포함,
data-toggle,data-target사용,ml-auto,mr-2유틸리티 클래스- Bootstrap 5 전용 기능(예:
data-bs-,ms-*,me-*,ps-*,pe-*클래스)이 발견되지 않음- 모든 템플릿에서 Bootstrap 5 고유 구문 미사용
현재 코드베이스의 모든 UI 컴포넌트가 Bootstrap 4.3.1 기준으로 올바르게 작동하므로, 제시된 "UI 깨짐 위험"은 실제 코드 상태에 반영되지 않습니다. 버전 다운그레이드가 발생했다면 이미 템플릿이 적절히 업데이트되었습니다.
Likely an incorrect or invalid review comment.
src/main/resources/static/js/app/index.js (2)
16-19: 로그아웃 버튼 이벤트 핸들러가 올바르게 구현되었습니다.앵커 태그의 기본 동작을 방지하고
logout()함수를 호출하는 구현이 정확합니다.preventDefault()를 사용하여 페이지 이동을 막고 프로그래매틱하게 로그아웃을 처리합니다.
110-130: 로그아웃 함수 구현이 우수합니다.CSRF 토큰을 메타 태그에서 읽어와 동적으로 폼을 생성하고 제출하는 구현이 정확하고 안전합니다:
- 메타 태그에서 CSRF 토큰과 파라미터 이름을 올바르게 추출
- 동적으로 폼을 생성하여 DOM에 불필요한 숨겨진 폼을 남기지 않음
- CSRF 토큰을 hidden input으로 추가하여 Spring Security의 CSRF 보호와 호환
/logout엔드포인트로 POST 제출이는 CSRF 보호가 활성화된 환경에서 로그아웃을 처리하는 표준 패턴입니다.
| flags: | | ||
| --min-instances=1 | ||
| --add-cloudsql-instances=${{ secrets.GCP_SQL_CONNECTION_NAME }} |
There was a problem hiding this comment.
🧩 Analysis chain
deploy-cloudrun 액션 v2와 flags 입력 호환성 검증
Cloud Run 배포 설정을 flags 블록으로 통합하셨습니다. google-github-actions/deploy-cloudrun@v2 액션이 이러한 flags 형식을 지원하는지 확인이 필요합니다.
다음 방법으로 액션 호환성을 확인해주세요:
또한 GCP_SQL_CONNECTION_NAME 시크릿이 GitHub 저장소의 Secrets에 올바르게 설정되어 있는지 확인해주세요.
🌐 Web query:
google-github-actions/deploy-cloudrun@v2 flags parameter documentation
💡 Result:
The flags input (optional) accepts a space-separated list of additional gcloud flags which the action appends to the underlying gcloud command:
- For services it is passed to
gcloud run deploy; for jobs it is passed togcloud jobs deploy. - Use quotes when a flag value itself contains spaces or other flags (example: --args="-X=123").
- The action does not parse/validate flags — you must ensure flags are valid for your gcloud version/subcommand.
Example:
with:
flags: '--add-cloudsql-instances=... "--args=-X=123"'
Sources: GitHub action README. [1][2]
Sources
[1] google-github-actions/deploy-cloudrun README (flags input).
[2] GitHub Marketplace — Deploy to Cloud Run (action inputs/flags).
YAML 플래그 형식 수정 필요
웹 검색 결과에 따르면 google-github-actions/deploy-cloudrun@v2의 flags 입력은 "space-separated list of additional gcloud flags"여야 합니다.
현재 코드의 문제점:
flags: |
--min-instances=1
--add-cloudsql-instances=${{ secrets.GCP_SQL_CONNECTION_NAME }}YAML 멀티라인 문자열(|)은 개행 문자로 구분된 형식을 생성하지만, gcloud 명령은 공백으로 구분된 플래그를 기대합니다. 이는 배포 실패 또는 플래그 무시로 이어질 수 있습니다.
수정 방법:
flags: '--min-instances=1 --add-cloudsql-instances=${{ secrets.GCP_SQL_CONNECTION_NAME }}'단일 라인으로 공백으로 구분하여 정의하거나, 여러 줄이 필요한 경우 > 폴드 연산자를 사용하여 개행을 공백으로 변환하세요.
🤖 Prompt for AI Agents
.github/workflows/cd.yml around lines 67 to 69: the flags value is using a YAML
block scalar (|) which preserves newlines, but
google-github-actions/deploy-cloudrun expects a space-separated list of gcloud
flags; replace the multiline block with a single space-separated string (or use
the folded scalar >) so flags become one line of space-separated options, e.g.
wrap the flags value in quotes and provide "--min-instances=1
--add-cloudsql-instances=${{ secrets.GCP_SQL_CONNECTION_NAME }}" so gcloud
receives the flags correctly.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/cd.yml (1)
67-69: 🚨 flags 입력이 여전히 잘못된 YAML 형식입니다.
google-github-actions/deploy-cloudrun@v2의flags입력은 공백으로 구분된 문자열이어야 하는데, YAML 멀티라인 블록 스칼라(|)를 사용하면 개행 문자가 보존되어 배포 실패 또는 플래그 무시로 이어질 수 있습니다.과거 리뷰에서 지적된 문제가 아직 미해결 상태입니다. gcloud 명령이 올바르게 파싱할 수 있도록 공백으로 구분된 단일 라인으로 수정하세요.
- flags: | - --min-instances=1 - --add-cloudsql-instances=${{ secrets.GCP_SQL_CONNECTION_NAME }} + flags: '--min-instances=1 --add-cloudsql-instances=${{ secrets.GCP_SQL_CONNECTION_NAME }}'또는 여러 줄이 필요한 경우
>폴드 연산자를 사용하여 개행을 공백으로 변환하세요:- flags: | - --min-instances=1 - --add-cloudsql-instances=${{ secrets.GCP_SQL_CONNECTION_NAME }} + flags: > + --min-instances=1 + --add-cloudsql-instances=${{ secrets.GCP_SQL_CONNECTION_NAME }}
🧹 Nitpick comments (3)
.github/workflows/cd.yml (1)
73-74: SPRING_PROFILES_ACTIVE=prod 추가는 적절하나, 후행 공백 라인 제거 권장프로덕션 환경에서 Spring 프로파일을 활성화하는 것은 바람직한 변경입니다. 다만 73줄 이후 74줄의 공백 라인은 불필요해 보이므로 제거하는 것을 권장합니다.
env_vars: | SPRING_PROFILES_ACTIVE=prod - DB_URL=jdbc:mysql://google/mydb?socketFactory=com.google.cloud.sql.mysql.SocketFactory&cloudSqlInstance=${{ secrets.GCP_SQL_CONNECTION_NAME }}&useSSL=false&serverTimezone=UTC&characterEncoding=UTF-8src/main/java/com/precourse/openMission/config/auth/SecurityConfig.java (2)
22-33: prod 전용 필터체인 분리로 이전 HTTPS 강제 이슈가 깔끔히 해결됨
@Profile("prod")를 통해 운영 환경에서만requiresChannel().anyRequest().requiresSecure()가 적용되도록 분리한 점이 좋습니다.- 공통 설정을
commonSecurityConfig(http)로 분리해 중복도 잘 제거되었습니다.운영 인프라 측면에서 한 가지는 확인해 보는 게 좋겠습니다.
- Cloud Run 같이 역프록시 뒤에서 TLS 종료가 되는 환경에서는, 애플리케이션 안에서는 HTTP 로 보이기 때문에
requiresSecure()가X-Forwarded-Proto등의 헤더 설정에 의존할 수 있습니다.server.forward-headers-strategy나ForwardedHeaderFilter등의 설정이 이미 되어 있다면 괜찮지만, 아니라면 HTTPS 요청임에도 불구하고 계속 리다이렉트가 걸리는 동작이 나올 수 있습니다.운영 프로파일(application-prod.properties 등)에서 이 부분이 이미 설정되어 있는지 한 번만 점검해 주세요.
43-69: 공통 보안 설정 추출로 prod/dev 일관성 및 유지보수성이 좋아짐
- 이전에 하나의
SecurityFilterChain안에 있던 설정을commonSecurityConfig(HttpSecurity)로 분리해 두 프로파일에서 재사용하는 구조가 깔끔합니다.authorizeHttpRequests,exceptionHandling,logout,oauth2Login이 모두 공통 경로에 모여 있어 정책 일관성도 잘 유지됩니다.- CSRF 를 기본값으로 유지하면서, 별도 GlobalModelAdvice/템플릿/JS 에서 토큰을 사용하는 현재 PR의 다른 변경들과도 자연스럽게 맞물립니다.
추후 필요하다면, API 전용 체인(예:
/api/**stateless, JWT 등)을 추가할 때 이 메서드를 더 잘게 나누는 정도의 리팩터링만 고려하면 될 것 같습니다. 지금 상태에서는 충분히 단순하고 읽기 쉬운 구조입니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/cd.yml(1 hunks)Dockerfile(1 hunks)src/main/java/com/precourse/openMission/config/auth/SecurityConfig.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Dockerfile
🔇 Additional comments (2)
src/main/java/com/precourse/openMission/config/auth/SecurityConfig.java (2)
7-7: 프로파일 분리를 위한@Profileimport 적절함
@Profile기반으로 보안 설정을 나누기 위한 필요한 import이고, 아래 prod/dev용 필터체인에 잘 활용되고 있습니다. 별도 이슈 없어 보입니다.
35-41: 비‑prod 환경용 필터체인 분리 방향 적절함
@Profile("!prod")로 기본/개발/테스트 프로파일에서는 HTTPS 강제를 끄고 동일한 공통 보안 설정만 적용하는 구조가 명확하고 이해하기 쉽습니다.- prod와 dev 필터체인이 동시에 활성화될 일이 없으므로
SecurityFilterChain중복 충돌 걱정도 없습니다.추후 다른 프로파일(e.g.
test,stage)를 추가하더라도 이 메서드가 그대로 재사용되므로 관리 측면에서도 무난해 보입니다.
1 participant
Summary by CodeRabbit
새로운 기능
보안 및 버그 수정
기타 개선
테스트